Skip to content

HDDS-15634. Avoid updating container delete transaction ID on SCM delete log append#10566

Merged
ivandika3 merged 4 commits into
apache:masterfrom
xichen01:HDDS-15634
Jun 28, 2026
Merged

HDDS-15634. Avoid updating container delete transaction ID on SCM delete log append#10566
ivandika3 merged 4 commits into
apache:masterfrom
xichen01:HDDS-15634

Conversation

@xichen01

@xichen01 xichen01 commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Reduce long-tail latency of OM allocateBlock

When SCM handles DeleteScmKeyBlocks requests, AllocateScmBlock can see high tail latency because SequenceIdGenerator#getNextId waits for SCMStateMachine to finish applying DeleteScmKeyBlocks's DeletedBlockLogStateManagerImpl#addTransactionsToDB.

Profiling shows a large part of DeletedBlockLogStateManagerImpl#addTransactionsToDB time is spent updating SCM-side ContainerInfo#deleteTransactionId.
This field is not used by current SCM business logic, while DN-side delete transaction tracking is maintained independently.

This commit removes the SCM-side ContainerInfo#deleteTransactionId update from addTransactionsToDB to reduce unnecessary StateMachine apply work and improve write tail latency.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-15634

How was this patch tested?

existing test

https://github.com/xichen01/ozone/actions/runs/27909174625

@xichen01 xichen01 changed the title HDDS-15634. Avoid updating container delete transaction ID on SCM del… HDDS-15634. Avoid updating container delete transaction ID on SCM delete log append Jun 21, 2026
ivandika3
ivandika3 reviewed Jun 22, 2026
View reviewed changes

@ivandika3 ivandika3 left a comment •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @xichen01 . Overall LGTM.

How about we deprecate the ContainerInfo deleteTransactionId entirely by marking proto and fields to be deprecated and adding some comments? Also we might just remove the ContainerManager#updateDeleteTransactionId

@xichen01

xichen01 commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @xichen01 . Overall LGTM.

How about we deprecate the ContainerInfo deleteTransactionId entirely by marking proto and fields to be deprecated and adding some comments? Also we might just remove the ContainerManager#updateDeleteTransactionId

deprecate ContainerInfo deleteTransactionId and ContainerStateManagerImpl#updateDeleteTransactionId

ivandika3
ivandika3 reviewed Jun 24, 2026
View reviewed changes

@ivandika3 ivandika3 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xichen01 Thanks for the update. LGTM +1.

@szetszwo

szetszwo commented Jun 25, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

... SequenceIdGenerator#getNextId waits for SCMStateMachine to finish applying DeleteScmKeyBlocks's DeletedBlockLogStateManagerImpl#addTransactionsToDB

Quick question: why getNextId will wait for addTransactionsToDB? Is it happen only if a new batch of ids is needed?

szetszwo
szetszwo reviewed Jun 25, 2026
View reviewed changes

@szetszwo szetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xichen01 , the change looks good. Just have a question (in my previous comment) and a comment inlined.

Comment thread ...dds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerStateManager.java Outdated
Comment on lines +239 to +241
@Deprecated
void updateDeleteTransactionId(Map<ContainerID, Long> deleteTransactionMap)
throws IOException;

@szetszwo szetszwo Jun 25, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is a local method, we could just delete it without deprecated it.

@xichen01

xichen01 commented Jun 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

... SequenceIdGenerator#getNextId waits for SCMStateMachine to finish applying DeleteScmKeyBlocks's DeletedBlockLogStateManagerImpl#addTransactionsToDB

Quick question: why getNextId will wait for addTransactionsToDB? Is it happen only if a new batch of ids is needed?

@szetszwo
Yes it is happen only if a new batch of ids is needed, to request the next batch ID, you must go through Ratis. SCMStateMachine's applyTransaction is also single-threaded; any slow transaction will block subsequent transactions.

This is a typical stack.

"IPC Server handler 23 on default port 9863" #289 [3407343] daemon prio=5 os_prio=0 cpu=581314.26ms elapsed=287858.01s tid=0x00007feb27451810 nid=3407343 waiting on condition  [0x00007feb8e0b9000]
   java.lang.Thread.State: TIMED_WAITING (parking)
	at jdk.internal.misc.Unsafe.park(java.base@21-sdi/Native Method)
	- parking to wait for  <0x000004161650b580> (a java.util.concurrent.CompletableFuture$Signaller)
	at java.util.concurrent.locks.LockSupport.parkNanos(java.base@21-sdi/LockSupport.java:269)
	at java.util.concurrent.CompletableFuture$Signaller.block(java.base@21-sdi/CompletableFuture.java:1866)
	at java.util.concurrent.ForkJoinPool.unmanagedBlock(java.base@21-sdi/ForkJoinPool.java:3780)
	at java.util.concurrent.ForkJoinPool.managedBlock(java.base@21-sdi/ForkJoinPool.java:3725)
	at java.util.concurrent.CompletableFuture.timedGet(java.base@21-sdi/CompletableFuture.java:1939)
	at java.util.concurrent.CompletableFuture.get(java.base@21-sdi/CompletableFuture.java:2095)
	at org.apache.hadoop.hdds.scm.ha.SCMRatisServerImpl.submitRequest(SCMRatisServerImpl.java:227)
	at org.apache.hadoop.hdds.scm.ha.SCMHAInvocationHandler.invokeRatisServer(SCMHAInvocationHandler.java:123)
	at org.apache.hadoop.hdds.scm.ha.SCMHAInvocationHandler.invokeRatis(SCMHAInvocationHandler.java:112)
	at org.apache.hadoop.hdds.scm.ha.SCMHAInvocationHandler.invoke(SCMHAInvocationHandler.java:74)
	at org.apache.hadoop.hdds.scm.ha.$Proxy14.allocateBatch(Unknown Source)
	at org.apache.hadoop.hdds.scm.ha.SequenceIdGenerator.getNextId(SequenceIdGenerator.java:142)
	at org.apache.hadoop.hdds.scm.block.BlockManagerImpl.newBlock(BlockManagerImpl.java:201)
	at org.apache.hadoop.hdds.scm.block.BlockManagerImpl.allocateBlock(BlockManagerImpl.java:177)
	at org.apache.hadoop.hdds.scm.server.SCMBlockProtocolServer.allocateBlock(SCMBlockProtocolServer.java:243)
	at org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocolServerSideTranslatorPB.allocateScmBlock(ScmBlockLocationProtocolServerSideTranslatorPB.java:222)
	at org.apache.hadoop.hdds.scm.protocol.ScmBlockLocationProtocolServerSideTranslatorPB.processMessage(ScmBlockLocationProtocolServerSideTranslatorPB.java:150)
	at 
//...

If the cluster's write and delete QPS are high, the end result will be a large number of long-tail allocateSCMBlocks.

image

szetszwo
szetszwo approved these changes Jun 27, 2026
View reviewed changes

@szetszwo szetszwo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 the change looks good.

@ivandika3 ivandika3 merged commit 1aed7dd into apache:master Jun 28, 2026
47 checks passed
@ivandika3

ivandika3 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Thanks @xichen01 for the patch and @szetszwo for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@szetszwo szetszwo szetszwo approved these changes
@ivandika3 ivandika3 ivandika3 approved these changes
@ashishkumar50 ashishkumar50 Awaiting requested review from ashishkumar50

Assignees

No one assigned

Labels

performance scm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xichen01 @szetszwo @ivandika3